Enable/fix test typing #2034

peter-hamilton · 2021-09-10T20:00:21Z

Description

Enable type-checking and fix type errors for a batch of packages.

Changes

react-async
- add missing cancelled property.
- changed noop() to match expected type.
react-bugsnag
- removed unused ReactPlugin plugin
react-form
- Change return type for mockField to allow for Field<undefined>
- cast onClick callbacks to any in a test where the event is programmatically triggered.
- add non-null assertions to controlled Root.find() returns.
- add missing argument to randomVariants() call
react-import-remote
- Used unsafe type casts to mock parts of the DOM.
react-graphql-universal-provider
- Removed unused variables
  - In one case, eslint disable-next-line no-new was suppressed for a test that with new ApolloLink()
react-hooks
- define argument types for mock functions
- add non-null assertions to controlled Root.find() returns
react-html
- fix typo nomodule -> noModule
react-network
- Correct function return type.
react-router
- Aligned the types returned by createDefaultHistory and createDefaultLocation. NOTE: These functions aren't used within Quilt so it's not clear how to test the changes.
react-server
- Import saddle-up matcher in src/tests/setup.ts
- cast Context.body as NodeJS.ReadableStream after createRender is called.
- Changed MockApp to return null instead of void in order to make it a valid JSX component.
react-testing
- Added missing options: MountOptions argument for mountWithContext being used in e2e.test.tsx. NOTE: It's possible options SHOULD be an optional parameter. If that's the case, .createMount typing should be changed instead.
- Used hard cast for a case where Node<unknown> was being compared to Element<{}>

Type of change

Checklist

I have added a changelog entry, prefixed by the type of change noted above (Documentation fix and Test update does not need a changelog as we do not publish new version)

BPScott

Looking very nice! I've dropped a few comments inline asking about any usage

BPScott · 2021-09-10T21:58:18Z

packages/react-form/src/hooks/list/tests/baselist.test.tsx

@@ -797,7 +797,7 @@ describe('useBaseList', () => {
            </>
          ))}

-          <button type="button" onClick={onNewDefault} />
+          <button type="button" onClick={onNewDefault as any} />


Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of onNewDefault to make it compliant?

I chose the brute force cast because the test doesn't deal with MouseEvents and I wanted to minimize changes to the original test. I followed the example of clickEvent() and changeEvent() used throughout react-forms tests. I agree it seems wonky, but I'm not sure how to avoid it without replacing Variant[] values with fake React.MouseEvent objects.

Uuugh yeah, it seems that this is doing weird things - it's triggering a button onClick with an argument that is nothing to do with a MouseEvent I'm kinda surprised that this works at all. This shouldn't be using a button - is should be some custom Event type that gets triggered and then we can control the type rather than using something weird and not fit for purpose .

I guess this is fine for now - but at some point we should work out how to rework this to use a custom event rather than repurposing the onClick event

BPScott · 2021-09-10T21:59:01Z

packages/react-form/src/hooks/list/tests/baselist.test.tsx

@@ -847,7 +847,7 @@ describe('useBaseList', () => {
          optionName: newDefaultOption,
          optionValue: newDefaultOptionValue,
        },
-      ]);
+      ] as any);


Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of newDefaultPrice/newDefaultOption/newDefaultOptionValue to make it compliant?

BPScott · 2021-09-10T21:59:08Z

packages/react-form/src/hooks/list/tests/baselist.test.tsx

@@ -881,7 +881,7 @@ describe('useBaseList', () => {
          optionName: newDefaultOption,
          optionValue: newDefaultOptionValue,
        },
-      ]);
+      ] as any);


Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of newDefaultPrice/newDefaultOption/newDefaultOptionValue to make it compliant?

BPScott · 2021-09-10T21:59:29Z

packages/react-form/src/hooks/list/tests/dynamiclist.test.tsx

@@ -372,7 +374,7 @@ describe('useDynamicList', () => {
            </>
          ))}

-          <button type="button" onClick={onNewDefault}>
+          <button type="button" onClick={onNewDefault as any}>


Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of onNewDefault to make it compliant?

BPScott · 2021-09-10T21:59:36Z

packages/react-form/src/hooks/list/tests/dynamiclist.test.tsx

@@ -430,7 +432,7 @@ describe('useDynamicList', () => {
          optionName: newDefaultOption,
          optionValue: newDefaultOptionValue,
        },
-      ]);
+      ] as any);


Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of newDefaultPrice/newDefaultOption/newDefaultOptionValue to make it compliant?

packages/react-testing/src/tests/element.test.tsx

BPScott

Dropped a comment regarding the element test inline.

I'm happy to leave the react-form tests as is for now.

peter-hamilton added 12 commits September 10, 2021 14:58

enable/fix test type checking for /react-async

cdc7878

enable/fix test type checking for /react-bugsnag

6af8c2d

enable/fix test type checking for /react-form

6d61996

enable/fix test type checking for /react-graphql-universal-provider

684eeee

enable/fix test type checking for /react-hooks

7d73af7

enable/fix test type checking for /react-html

8b1b535

enable/fix test type checking for /react-remote

d66ebdc

enable/fix test type checking for /react-network

36c03de

enable/fix test type checking for /react-router

61b4a94

enable/fix test type checking for /react-server

3bfccdc

enable/fix test type checking for /react-testing

b21f65c

Update changelog

dc005ab

peter-hamilton requested a review from a team September 10, 2021 20:14

peter-hamilton marked this pull request as ready for review September 10, 2021 20:14

BPScott reviewed Sep 10, 2021

View reviewed changes

BPScott approved these changes Sep 13, 2021

View reviewed changes

peter-hamilton and others added 2 commits September 13, 2021 16:21

Remove hard cast

1f0a82b

Merge branch 'main' into add-test-typing

7ced21b

peter-hamilton merged commit dd41598 into main Sep 14, 2021

peter-hamilton deleted the add-test-typing branch September 14, 2021 17:06

shopify-shipit bot had a problem deploying to production September 14, 2021 19:04 Failure

shopify-shipit bot temporarily deployed to production September 14, 2021 20:47 Inactive

shopify-shipit bot temporarily deployed to gem November 16, 2021 02:42 Inactive

shopify-shipit bot temporarily deployed to michenly-beta November 25, 2021 02:39 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable/fix test typing #2034

Enable/fix test typing #2034

peter-hamilton commented Sep 10, 2021 •

edited

Loading

BPScott left a comment

BPScott Sep 10, 2021

peter-hamilton Sep 13, 2021

BPScott Sep 13, 2021

BPScott Sep 10, 2021

BPScott Sep 10, 2021

BPScott Sep 10, 2021

BPScott Sep 10, 2021

BPScott left a comment

Enable/fix test typing #2034

Enable/fix test typing #2034

Conversation

peter-hamilton commented Sep 10, 2021 • edited Loading

Description

Changes

Type of change

Checklist

BPScott left a comment

Choose a reason for hiding this comment

BPScott Sep 10, 2021

Choose a reason for hiding this comment

peter-hamilton Sep 13, 2021

Choose a reason for hiding this comment

BPScott Sep 13, 2021

Choose a reason for hiding this comment

BPScott Sep 10, 2021

Choose a reason for hiding this comment

BPScott Sep 10, 2021

Choose a reason for hiding this comment

BPScott Sep 10, 2021

Choose a reason for hiding this comment

BPScott Sep 10, 2021

Choose a reason for hiding this comment

BPScott left a comment

Choose a reason for hiding this comment

peter-hamilton commented Sep 10, 2021 •

edited

Loading